-
Notifications
You must be signed in to change notification settings - Fork 379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SonarCloud PR Analysis #9123
SonarCloud PR Analysis #9123
Conversation
The latest updates on your projects. Learn more about Vercel for Git βοΈ 4 Skipped Deployments
|
dc424c7
to
92470e3
Compare
.github/workflows/pr-analysis.yml
Outdated
run: | | ||
pnpm i --filter="live-mobile..." --filter="ledger-live" --no-frozen-lockfile --unsafe-perm | ||
|
||
- name: Build desktop dependencies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we really need to build deps for LLM and LLD ? sonar analyzer don't need these deps imo .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to build them before running the tests below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then you can compact them under the installation process while they share the same condition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right π
.github/workflows/pr-analysis.yml
Outdated
run: | | ||
cat apps/ledger-live-desktop/coverage/lcov.info apps/ledger-live-mobile/coverage/lcov.info > ./lcov.info | ||
|
||
- name: Copy test execution files for LLD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can do this on LLD unit test generation
.github/workflows/pr-analysis.yml
Outdated
run: | | ||
cat apps/ledger-live-desktop/coverage/lld-sonar-executionTests-report.xml > ./lld-sonar-executionTests-report.xml | ||
|
||
- name: Copy test execution files for LLM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can do this on LLM unit test generation
dc50914
to
30a6583
Compare
apps/ledger-live-desktop/test
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to delete i guess. (i think you test with it so you can resolve my thread once done)
621746b
to
f4b2cb1
Compare
|
f4b2cb1
to
b007949
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM nice work for a v1
β Checklist
npx changeset
was attached.π Description
Add a workflow that triggers a SonarQube Cloud Pull Request analysis on all PR targetting develop
Demo
Mobile Only (14 minutes to run)
Mobile and Desktop (24 minutes to run)
β Context
π§ Checklist for the PR Reviewers